feat(workbench): add Audit Database support - #158
Conversation
Adds spec.workbench.experimentalFeatures.auditDatabaseEnabled, which provisions a second Postgres database (distinct from the internal database) via the same EnsureDatabaseExists/PostgresDatabase machinery already used for the internal database, and renders audit-database.conf pointing Workbench at it. Also sets package-audit=1 in rserver.conf. The audit role's password is resolved server-side (via the existing product.FetchSecret helper, keyed by a new "dev-audit-db-password" Site secret entry) and written directly into audit-database.conf's Password field, since that file -- like database.conf -- is rendered into a Kubernetes Secret rather than a ConfigMap. A dedicated role/password is required because WORKBENCH_POSTGRES_PASSWORD is shared between database.conf and audit-database.conf and cannot carry two different passwords for two different roles. workbench-api-admin-enabled/-super-admin-enabled need no changes here: they're already set whenever ApiSettings.Enabled is true. See: https://docs.posit.co/ide/server-pro/admin/database/audit/database.html
|
Claude finished @zachhannum's task in 1m 56s —— View job PR Review
Overall this is a clean, well-scoped feature that follows existing patterns. Two issues worth addressing before merge: Critical None. Important
Suggestion
|
| return ctrl.Result{}, err | ||
| } | ||
|
|
||
| auditDbName := invalidCharacters.ReplaceAllString(auditComponentName, "_") |
There was a problem hiding this comment.
The auditDbName is {name}-workbench-audit after sanitization (e.g. my-site-workbench-audit). Postgres identifier max length is 63 bytes; the dbName for the primary database is {name}_workbench, which is already fine. But adding -audit appends 6 more characters. If a Workbench name is close to the 256-character API limit this would overflow. That limit is far higher than Postgres allows, so this is worth truncating — or at minimum, document that names must stay short enough to accommodate the suffix.
The main database path has the same potential issue, but adding 6 more characters here makes it more likely to trip in practice for longer names.
| Host string `json:"host,omitempty"` | ||
| Username string `json:"username,omitempty"` | ||
| Password string `json:"password,omitempty"` | ||
| AutoCreate string `json:"auto-create,omitempty"` |
There was a problem hiding this comment.
AutoCreate is defined in the struct but never set in the controller. If this field doesn't need to be user-configurable and the operator always manages database creation via EnsureDatabaseExists, it should be removed to avoid confusion. If there's a scenario where it should be set, add the logic for it.
- Truncate the audit component name to Postgres's 63-byte identifier limit before deriving the database/role name from it, so a long Workbench name plus the "-audit" suffix can't silently overflow. - Remove WorkbenchAuditDatabaseConfig.AutoCreate: dead field, never set by the controller (the operator always provisions the audit database itself via EnsureDatabaseExists). - Add TestSiteAuditDatabaseEnabled / TestSiteAuditDatabaseDisabledByDefault covering the Site -> Workbench propagation path, mirroring the existing AuditedJobs/ForceAdminUiEnabled test pattern. - Regenerate CRDs: also picks up a stale chronicleBuiltinEnabled entry in core.posit.team_sites.yaml left over from the earlier branch rebase (removed from the Go types, but not re-synced to the CRD YAML at the time).
|
Addressed both review findings in 24968a0:
Also added the suggested While regenerating CRDs I also caught a stale Full |
make helm-generate regenerates dist/chart/templates/crd/*.yaml from config/crd/bases/*.yaml; the CI "Verify Helm chart is in sync with kustomize" check caught that this hadn't been run after the site/ workbench CRD changes for Audit Database support.
make generate-client (kube_codegen) hadn't been run after the API changes for Audit Database support; CI's "Assert no diff" check caught the drift in client-go/applyconfiguration/.
package-audit=1 and "audit database configured" are two independent prerequisites for the Package Audit Read API (see rstudio-pro's wiki/features/package-audit-read-api.qmd) -- not the same feature. AuditDatabaseEnabled provisioning the audit database should not also silently flip on package scanning; other consumers of the audit database (general session/usage history) don't want that side effect. Removes WorkbenchRServerConfig.PackageAudit and the auto-set on AuditDatabaseEnabled. package-audit=1 can be set independently via the existing free-form rserver.conf additionalConfigs escape hatch on consumers that want it -- it doesn't need a dedicated CRD field.
Lytol
left a comment
There was a problem hiding this comment.
Great! Let me know if you want any help with testing this on an internal workload.
|
@Lytol had to make a tweak to fix the failing PR checks, would appreciate another quick review! |
# [1.31.0](v1.30.2...v1.31.0) (2026-08-27) ### Features * **workbench:** add Audit Database support ([#158](#158)) ([c325a72](c325a72))
Summary
spec.workbench.experimentalFeatures.auditDatabaseEnabled, which provisions a second Postgres database (distinct from the internal database) via the sameEnsureDatabaseExists/PostgresDatabasemachinery already used for the internal database, and rendersaudit-database.confpointing Workbench at it.product.FetchSecrethelper, keyed by a newdev-audit-db-passwordSite secret entry) and written directly intoaudit-database.conf'sPasswordfield — that file, likedatabase.conf, is rendered into a Kubernetes Secret rather than a ConfigMap, so no credentials land in plaintext CR/ConfigMap data.WORKBENCH_POSTGRES_PASSWORDis shared betweendatabase.confandaudit-database.confand cannot carry two different passwords for two different roles.dev-audit-db-passwordkey to their Site secret before enabling this flag (mirrors the existingdev-db-passwordkey).Test plan
make generate/make generate-all(deepcopy + client-go + openapi) /make manifests/make helm-generate/make copy-crds/make verify-crds— all regenerated and in syncgo build ./...,go vet ./...make go-test— full suite green, including envtest-based reconciler tests ininternal/controller/coremake test-kustomize,make helm-lint,make helm-template— no driftTestWorkbenchSecretConfig_GenerateSecretData(audit-database.conf rendering),TestSiteAuditDatabaseEnabled/TestSiteAuditDatabaseDisabledByDefault(Site → Workbench propagation)